Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] spell check, formatting, code cleanup #212

Merged
merged 11 commits into from
Apr 21, 2020
Merged

[WIP] spell check, formatting, code cleanup #212

merged 11 commits into from
Apr 21, 2020

Conversation

sungam3r
Copy link
Member

@sungam3r sungam3r commented Apr 3, 2020 •
edited
Loading

This is the cleanup I did while reviewing the code briefly.

@sungam3r
Copy link
Member Author

sungam3r commented Apr 4, 2020

@rose-a Could you help me with tests? I see that github actions works only on my branch, but not on this PR. Locally tests run without errors. I understand that I’ve made a lot of changes, but I still would like to understand whether the tests fall because of them or not.

@rose-a
Copy link
Collaborator

rose-a commented Apr 4, 2020

Hi, nice to see you investing in this project!

The issue with the tests failing (randomly) on GitHub Actions is the thing which currently bothers me the most, and where I'm running out of ideas why they keep failing. (so it's currently nothing "special" in your case)...

I'm trying to keep track of this in #161 and I'm currently overhauling the observable test helpers in #213 (and planning to propose a PR to FluentAssertions if those tests finally run).

Seems to be a threading issue where the reception of the notification pushed by the observable is blocked by the test waiting for it, which is also somehow dependent on the machine it runs on....

@rose-a
Copy link
Collaborator

rose-a commented Apr 4, 2020

btw. please target develop

@sungam3r
Copy link
Member Author

sungam3r commented Apr 4, 2020

I will wait for your changes and then resolve conflicts.

@sungam3r sungam3r changed the title spell check, formatting, code cleanup [WIP] spell check, formatting, code cleanup Apr 4, 2020
@sungam3r
Copy link
Member Author

sungam3r commented Apr 4, 2020

изображение

@sungam3r
Merge branch 'master' of https://github.com/graphql-dotnet/graphql-cl…
6e50381 
…ient into code-cleanup

# Conflicts:
#	src/GraphQL.Client.Abstractions.Websocket/GraphQL.Client.Abstractions.Websocket.csproj
#	src/GraphQL.Client.Serializer.SystemTextJson/GraphQL.Client.Serializer.SystemTextJson.csproj
#	src/GraphQL.Client/GraphQL.Client.csproj
#	src/GraphQL.Client/GraphQLHttpException.cs
#	src/GraphQL.Primitives/GraphQL.Primitives.csproj
#	src/src.props
#	tests/GraphQL.Client.Serializer.Tests/GraphQL.Client.Serializer.Tests.csproj
#	tests/GraphQL.Client.Tests.Common/Helpers/ObservableTester.cs
#	tests/GraphQL.Integration.Tests/GraphQL.Integration.Tests.csproj
#	tests/GraphQL.Integration.Tests/WebsocketTests/Base.cs
#	tests/GraphQL.Primitives.Tests/GraphQL.Primitives.Tests.csproj
@sungam3r
Copy link
Member Author

btw. please target develop

There is no develop anymore.

@sungam3r
Copy link
Member Author

@rose-a Done. Tests success locally.

@rose-a
Copy link
Collaborator

rose-a commented Apr 21, 2020

There is no develop anymore.

Correct, moved to "GitHubFlow" to simplify things...

@rose-a rose-a merged commit 4e07ff1 into graphql-dotnet:master Apr 21, 2020
@sungam3r sungam3r deleted the code-cleanup branch April 21, 2020 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rose-a rose-a rose-a approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sungam3r @rose-a